[PM-33162] Refactor user key rotation#7201
Conversation
|
New Issues (4)Checkmarx found the following issues in this Pull Request
Fixed Issues (3)Great job! The following issues were fixed in this Pull Request
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7201 +/- ##
==========================================
+ Coverage 57.66% 57.68% +0.01%
==========================================
Files 2037 2037
Lines 89701 89719 +18
Branches 7979 7976 -3
==========================================
+ Hits 51728 51751 +23
+ Misses 36117 36112 -5
Partials 1856 1856 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| [HttpPost("key-management/rotate-user-account-keys")] | ||
| public async Task RotateUserAccountKeysAsync([FromBody] RotateUserAccountKeysAndDataRequestModel model) | ||
| public async Task PasswordChangeAndRotateUserAccountKeysAsync([FromBody] RotateUserAccountKeysAndDataRequestModel model) |
There was a problem hiding this comment.
KM team, I've realized changing this method name will effect the SDK name of the generated API bindings. Do we think that is worth it or should I just revert it back to the original name?
There was a problem hiding this comment.
I'm OK with it, it just means we need to fix the breaking API bindings PR. I.e we run the API bindings automation on the SDK repo and do a rename on that branch, and review. Should be fairly low effort?
|
quexten
left a comment
quexten
left a comment
There was a problem hiding this comment.
Looks good from my side; Only one question
src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataAndAuthenticationModel.cs
Show resolved
Hide resolved
Thomas-Avery
dismissed stale reviews from quexten, ike-kottlowski, and enmande
via
73f6b59
|
https://github.com/bitwarden/server/pull/7230/changes changed the behavior of |
|
6 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33162
📔 Objective
The objective of this PR is to refactor the current change password and user key rotation endpoint to use, via composition, a new base data model. This is preparation of adding a new endpoint to support none password change userKey rotation that will share the same base data model and processing logic.
Note to Auth reviewers:
The only auth code changes was moving
MasterPasswordUnlockAndAuthenticationDataModelto KM ownership.MasterPasswordUnlockAndAuthenticationDataModelthe request model is only used on the KM key rotation endpoint. Let me know if you don't agree and want to still retain ownership.